-
Notifications
You must be signed in to change notification settings - Fork 4.6k
List Imported Services Changes #23133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
List Imported Services Changes #23133
Conversation
| return 0 | ||
| } | ||
|
|
||
| if c.format == JSONFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wont switch case be better? In case we add more formats in future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
| return 0 | ||
| } | ||
|
|
||
| c.UI.Output(formatImportedServices(filteredServices)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be default case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| return uniqueImportedServices[i].peer < uniqueImportedServices[j].peer | ||
| }) | ||
|
|
||
| return uniqueImportedServices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are looping over a lot services, intentions throughout this code piece, I feel it could be a performance issue, should we rather paginate the response? that way we wont have to process each of the intention and service, etc. for every call.
Since this does not hamper the core functionality, it is not a blocker, however worth discussing once and can be planned for phase 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will discuss and plan it for Phase 2
Description
The services can be imported across partitions with in a local data center. However there is no API to List the services imported by the downstream partition. The new API /v1/imported-services Lists the imported services across partitions with in a local datacenter.
Testing & Reproduction steps
Step 1: Create consul server agents
Step 2: Create partitions
consul partition create -name upstream -description "Upstream partition with services" -token=rootconsul partition create -name downstream -description "Downstream partition consuming services" -token=rootStep 3: Run separate consul client agents for each partitions.
Step 4: Register the services in partitions
consul services register -token=root -http-addr=http://localhost:8510 payment-service.jsonpayment-service.json
consul services register -token=root -http-addr=http://localhost:8520 billing-service.jsonbilling-service.json
consul config write -http-addr=http://localhost:8520 -token=root exported-services.hclexported-services.hcl
Step 5: Below curl will help in fetching the imported services
Links
PR Checklist
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.